Skip to content

Refactor lint workflows: composite action for fork-safe changed-file detection, split workflow_dispatch#6

Closed
Copilot wants to merge 9 commits intomasterfrom
copilot/refactor-markdown-link-check-workflow
Closed

Refactor lint workflows: composite action for fork-safe changed-file detection, split workflow_dispatch#6
Copilot wants to merge 9 commits intomasterfrom
copilot/refactor-markdown-link-check-workflow

Conversation

Copy link

Copilot AI commented Mar 2, 2026

Changed-file detection in the lint workflows used origin/${{ github.base_ref }} which resolves incorrectly for fork PRs. This replaces it with a composite action using the exact base/head SHAs from the PR event, and splits workflow_dispatch handling into dedicated workflows.

  • This PR handles the issue and requires no additional PRs.
  • You have validated the need for this change.

What did this PR accomplish?

  • Added .github/actions/get-changed-files/action.yml — composite action that:
    • Accepts base_sha, head_sha, base_repo, repo_path, and exclude_pattern inputs
    • Idempotently adds an upstream remote and shallow-fetches the exact base SHA (works for fork PRs where origin only contains fork commits)
    • Outputs all_changed and files_updated as newline-separated multiline values (using GitHub Actions heredoc <<EOF delimiter syntax) for robust filename handling; writes a ## Changed files summary
  • Updated md-link-check.yml (PR-only, two checkouts):
    • PR head is checked out to the workspace root (provides the composite action files and PR content); OWASP base is checked out to base/
    • Replaced the inline bash "Get Changed Files" step with a call to the composite action using github.event.pull_request.base.sha / head.sha with repo_path: .
    • Removed workflow_dispatch trigger and the full-repo "Repository Link Check" step (moved to dedicated workflow)
    • Pinned the OWASP base checkout ref to master
    • Consuming step uses readarray -t to safely read multiline outputs into a Bash array and iterates with quoted array expansion, avoiding fragile word-splitting
  • Added .github/workflows/md-link-check-full.yml — new workflow_dispatch-only workflow for full-repository link checking, extracted from the original combined workflow
  • Updated md-lint-check.yml (two checkouts):
    • PR head checked out to workspace root; OWASP base checked out to base/
    • Pinned the OWASP base checkout ref to master
    • Replaced the inline git diff origin/$base_ref changed-files block with a call to the composite action (repo_path: .) for fork-safe detection
    • Consuming step uses readarray -t + quoted array expansion for robust filename handling
  • Updated md-textlint-check.yml with the same set of changes as md-lint-check.yml, including readarray -t array handling and explicit shell: bash
  • Updated .github/workflows/README.md to reflect the two-checkout model, the composite action for changed-file detection, the new md-link-check-full.yml workflow, and the removal of workflow_dispatch from md-link-check.yml

Thank you for your contribution!

Original prompt

Create a PR in kingthorin/wstg (base branch master) that refactors ONLY the Markdown link check workflow to reuse PR-accurate changed-file detection via a composite action.

Context:

  • Current workflows use repeated inline bash to compute changed files using native git diff.
  • Prior experiments suggest reusable workflows are awkward due to separate job context and existing multi-checkout layout (base/ and pr/).
  • We want Option 1 semantics: compute changed files exactly as GitHub PR shows, using base/head SHAs from the PR event (not origin/base_ref).
  • This must work for PRs from forks.

Tasks:

  1. Add a composite action at .github/actions/get-changed-files/action.yml that:

    • Inputs: repo_path (default: pr), base_sha (required), head_sha (required), base_repo (default to current repo), and an optional exclude pattern defaulting to '^.github/'.
    • Adds an 'upstream' remote pointing at https://github.com/${base_repo}.git (idempotently).
    • Fetches base_sha from upstream (ensure the commit exists locally).
    • Ensures head_sha exists locally (it should via checkout, but handle defensively).
    • Computes:
      • all_changed: git diff --name-only --diff-filter=d ${base_sha}...${head_sha} filtered to exclude .github
      • files_updated: all_changed filtered to *.md
    • Outputs both as space-separated lists (trim whitespace, no empty tokens).
    • Appends a "## Changed files" section to $GITHUB_STEP_SUMMARY listing the markdown files (one per line).
  2. Update .github/workflows/md-link-check.yml ONLY:

    • Replace the existing inline "Get Changed Files" step with a call to the composite action.
    • Use github.event.pull_request.base.sha and github.event.pull_request.head.sha for inputs.
    • Continue producing the same downstream environment variables used by the workflow (FILES, ALL_CHANGED) using the composite action outputs.
    • Keep existing behavior for workflow_dispatch (can remain, but changed-files logic should run only on pull_request).
  3. Keep behavior equivalent to current link check:

    • Copy all changed files from pr/ to base/ prior to running link checking so relative link targets resolve.
    • Only run checks on changed markdown files.
  4. Add minimal documentation/comments in md-link-check.yml explaining why base/head SHAs are used (fork-safe, PR-accurate).

Acceptance:

  • PR contains composite action and md-link-check.yml updated to use it.
  • Link check workflow continues to work on PRs; changed file detection is based on base/head SHAs and works for forks.

This pull request was created from Copilot chat.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…eck.yml

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor Markdown link check workflow to reuse changed-file detection Refactor md-link-check: composite action for fork-safe changed-file detection Mar 2, 2026
@kingthorin
Copy link
Owner

@copilot Update PR #6 to pin OWASP base checkout to master for pull_request

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 2, 2026

@copilot Update PR #6 to pin OWASP base checkout to master for pull_request

Done in 0b10294 — replaced ${{ github.base_ref || 'master' }} with the literal master.

@kingthorin
Copy link
Owner

@copilot Please update the PR branch (copilot/refactor-markdown-link-check-workflow) to ensure the repository containing the local action is checked out before the workflow tries to use ./.github/actions/get-changed-files.

Insert the step below as the first step of the job (or at least before the 'Get Changed Files' step) in .github/workflows/md-link-check.yml.
Use commit message: 'Ensure workflow repository is checked out before using local action'.
Push the commit to the same PR branch so the PR updates and CI can re-run.
# Please insert this step into the job's steps BEFORE the step that uses the local action ./.github/actions/get-changed-files
- name: Checkout workflow repository (self)
  uses: actions/checkout@v3
  with:
    # ensure we get the PR head commit that contains the composite action files
    ref: ${{ github.event.pull_request.head.sha }}
    fetch-depth: 0

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 2, 2026

@copilot Please update the PR branch (copilot/refactor-markdown-link-check-workflow) to ensure the repository containing the local action is checked out before the workflow tries to ...

Done in aca9246 — added "Checkout workflow repository (self)" as the first step using the existing pinned SHA (actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6) and ${{ github.event.pull_request.head.sha || github.sha }} so it works for both pull_request and workflow_dispatch events.

@kingthorin
Copy link
Owner

@copilot proceed with splitting the workflow_dispatch handling into another workflow.
Also update the other two lint workflows, using our changed files change, and splitting for workflow_dispatch if/as needed.

…flows to use composite action

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 2, 2026

@copilot proceed with splitting the workflow_dispatch handling into another workflow.
Also update the other two lint workflows, using our changed files change, and splitting for workflow_dispatch if/...

Done in ef7fda1:

  • md-link-check.yml: Removed workflow_dispatch trigger and the "Repository Link Check" step; cleaned up now-redundant if: github.event_name == 'pull_request' guards (the workflow is PR-only).
  • md-link-check-full.yml (new): workflow_dispatch-only full-repo link check with the same Node/npm/markdown-link-check setup.
  • md-lint-check.yml and md-textlint-check.yml: Added "Checkout workflow repository (self)" as first step, pinned OWASP base checkout to master, and replaced the inline git diff origin/$base_ref changed-files block with a call to ./.github/actions/get-changed-files (fork-safe). Neither had workflow_dispatch, so no split was needed.

Copilot AI changed the title Refactor md-link-check: composite action for fork-safe changed-file detection Refactor lint workflows: composite action for fork-safe changed-file detection, split workflow_dispatch Mar 2, 2026
@kingthorin
Copy link
Owner

@copilot update the changes on this branch so that I can open a PR against OWASP/wstg. Remove anything that was added to allow testing with stacked PRs. Consider whether all three checkouts are necessary or if we can work practically with just two.

…e file path references

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 3, 2026

@copilot update the changes on this branch so that I can open a PR against OWASP/wstg. Remove anything that was added to allow testing with stacked PRs. Consider whether all three ch...

Done in 66af689:

  • Stacked-PR artifacts removed: dropped the - copilot/refactor-markdown-link-check-workflow branch entry from both md-lint-check.yml and md-textlint-check.yml.
  • Three checkouts → two: removed the separate "Checkout workflow repository (self)" step in all three PR workflows. The "Checkout PR" step now checks out the PR head to the workspace root (no path:), which makes .github/actions/get-changed-files available for the composite action call AND provides the PR files directly. "Checkout Base" (OWASP/wstg at base/) remains unchanged. Updated repo_path: . in each composite action call and dropped pr/ prefixes from downstream file references.

@kingthorin
Copy link
Owner

@copilot update the workflows readme

…, and new md-link-check-full.yml

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 3, 2026

@copilot update the workflows readme

Done in 3d51951 — updated .github/workflows/README.md to:

  • Replace the pr/ two-path checkout description with the new two-checkout model (PR head at workspace root, base at base/)
  • Reference the .github/actions/get-changed-files composite action instead of inline git diff for all three PR lint workflows
  • Add a new md-link-check-full.yml section (workflow_dispatch full-repo check)
  • Remove the workflow_dispatch trigger mention from md-link-check.yml's entry

@kingthorin
Copy link
Owner

@copilot update the composite action to emit multiline outputs (preserving newlines) and adjusted every workflow step that consumes those outputs to safely read them into a Bash array with readarray, then iterate using quoted array expansion. That makes filename handling robust (spaces/newlines) and avoids fragile word-splitting.

…workflows

Co-authored-by: kingthorin <7570458+kingthorin@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 3, 2026

@copilot update the composite action to emit multiline outputs (preserving newlines) and adjusted every workflow step that consumes those outputs to safely read them into a Bash arra...

Done in c521425:

  • action.yml: replaced tr '\n' ' ' | xargs with heredoc <<EOF blocks so files_updated and all_changed are emitted as newline-separated multiline outputs; updated descriptions accordingly.
  • md-link-check.yml, md-lint-check.yml, md-textlint-check.yml: each consuming step now opens with readarray -t files_arr <<< "$FILES" (and readarray -t all_arr <<< "$ALL_CHANGED" in the link-check step) and iterates with "${arr[@]}" plus a [ -z "$FILE" ] && continue guard for the empty-array case.

@kingthorin
Copy link
Owner

@copilot update the readme if necessary.

Copy link
Author

Copilot AI commented Mar 3, 2026

@copilot update the readme if necessary.

The README doesn't reference the output format (space-separated vs. newline-separated) — it describes the composite action at a behavioral level, which is unchanged. No update needed.

@kingthorin kingthorin closed this Mar 3, 2026
@kingthorin kingthorin deleted the copilot/refactor-markdown-link-check-workflow branch March 3, 2026 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants